Distribute a modern .vsix via VSIX Gallery and SSMS Gallery#2
Conversation
Drop never-built System.Text.Json family from the DLL whitelist and manifest.json (the codebase deliberately avoids System.Text.Json), and add the five System.* DLLs that actually ship to manifest.json files[]. No change to the loose-file payload — the phantom DLLs were never produced. Makes manifest.json accurate for VSIX catalog installs.
Zip the already-assembled SSMS22 layout (built from SqlPilotDlls.txt) plus an OPC [Content_Types].xml into SqlPilot-<tag>.vsix and attach it to the release. Built from a separate staging copy so the loose-file payload ZIP is unchanged. No csproj/build change — the VSSDK CreateVsixContainer target does not wire up in this SDK-style project.
Upload SqlPilot-<tag>.vsix to both galleries after the GitHub release. Gated to non-prerelease tags. Manage tokens come from repo secrets VSIXGALLERY_TOKEN / SSMSGALLERY_TOKEN.
Add VSIX Gallery badge + install note (SSMS 22 only; 18/20 stay installer-only), document the additive .vsix release step and the [21.0,23.0) target/bump policy, and record that CreateVsixContainer is inert in this SDK-style project (use the zip-the-SSMS22-folder approach).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds VSIX artifact generation to the release workflow by staging the SSMS22 folder, writing ChangesVSIX artifact, dependency manifest, and publishing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 230-236: After the first curl.exe call that publishes to the
Visual Studio Gallery (the one using the $vg variable and VSIXGALLERY_TOKEN),
add an explicit check for $LASTEXITCODE and throw an error if the exit code
indicates failure. Similarly, after the second curl.exe call that publishes to
SSMS Gallery (the one using the $sg variable and SSMSGALLERY_TOKEN), add another
$LASTEXITCODE check to catch any failure in that upload. This ensures that if
either gallery upload fails, the workflow step will fail immediately rather than
allowing a subsequent successful upload to mask a prior failure.
- Around line 168-177: The PowerShell here-string syntax `@' ... '@` used to
define the XML contentTypes variable is breaking YAML parsing because YAML
doesn't recognize PowerShell syntax. Replace the here-string delimiters with
YAML's literal block scalar syntax (using `|-` or `|`) to properly define the
multi-line XML content while maintaining valid YAML structure. The XML content
can remain unchanged, but the outer PowerShell here-string syntax needs to be
replaced with YAML block scalar notation so the workflow file can be parsed
correctly by actionlint and yamllint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 982f3040-d1b9-47d6-8534-00f946f5f85f
📒 Files selected for processing (7)
.github/workflows/release.ymlREADME.mdbuild/SqlPilotDlls.txtdocs/ARCHITECTURE.mddocs/INSTALL.mddocs/SSMS_INTEGRATION_NOTES.mdsrc/SqlPilot.Package/manifest.json
💤 Files with no reviewable changes (1)
- build/SqlPilotDlls.txt
…ring The @'...'@ here-string placed its content at column 0, which terminates the YAML run: literal block and made the workflow file invalid (GitHub rejected it before any job ran). Build the XML via an indented array-join instead so every line stays within the block.
Check $LASTEXITCODE after each curl so a failed VSIX Gallery upload isn't masked by a successful SSMS Gallery upload (pwsh does not fail the step on a native non-zero exit).
The modern SSMS 22 VSIXInstaller rejects a .vsix that contains the loose-file deployment descriptors (catalog.json/manifest.json) with 'InvalidExtensionManifestException: version Invalid'. Strip them from the .vsix staging copy only; the payload ZIP still ships them for the file-copy installer. Verified: stripped .vsix installs with exit 0 to the per-user SSMS 22 Extensions store. Documented in SSMS_INTEGRATION_NOTES.
Adds discoverability via the VSIX Gallery and the new SSMS Gallery (see #1) by producing a modern (SSMS 21–22)
.vsixas a purely additive output of the existing release pipeline.Approach
A
.vsixis just an OPC ZIP.release.ymlalready assembles theSSMS22/layout (DLLs fromSqlPilotDlls.txt+pkgdef+PackageManifest 2.0manifest +manifest.json+catalog.json), so we zip a separate staging copy of it plus a generated[Content_Types].xml. Payload parity with the installer is automatic via the shared single-source-of-truth DLL list.The MSBuild
<CreateVsixContainer>route was empirically ruled out — it's inert in this SDK-style project (the VSSDK target never wires up becausesource.extension.vsixmanifestis only auto-globbed), so it produces no.vsix. Documented inSSMS_INTEGRATION_NOTES.md.Existing flow is unchanged
.vsixis built from a separate$vsixStagingdir, so the loose-fileSSMS22/folder and the payload ZIP are byte-for-byte identical to before.Deploy-Dev.ps1, the installers, or the installer ZIP..vsixis a valid 15-entry OPC package (manifest parses asPackageManifest 2.0, payload == the 10 shipped DLLs, no vendored Microsoft DLLs or PDBs leaked).Commits
System.Text.Json-family entries fromSqlPilotDlls.txtand fixesmanifest.jsonfiles[]to list the true 10 shipped DLLs. (Pre-existing drift; the loose-file payload is unaffected since those DLLs were never produced.)release.ymlzips theSSMS22/layout +[Content_Types].xmlintoSqlPilot-<tag>.vsixand attaches it to the release.Before this publishes (manual prerequisites)
SqlPilot-<tag>.vsixon a real SSMS 22 and confirm SQL Pilot loads (no automated test installs into live SSMS).VSIXGALLERY_TOKENandSSMSGALLERY_TOKEN(the per-galleryX-Manage-Tokenvalues).vX.Y.Ztag (no-suffix) to trigger the gallery publish. Prerelease tags build/attach the.vsixbut skip publishing.The modern manifest targets SSMS
[21.0,23.0); bump the upper bound per release after testing a new SSMS rather than open-ending it.Summary by CodeRabbit